feat(optimizer): [0/N] Optimizer Data Model#527
feat(optimizer): [0/N] Optimizer Data Model#527mkuchenbecker wants to merge 6 commits intolinkedin:mainfrom
Conversation
Introduces the optimizer service module with: - MySQL/H2 schema for table_operations, table_stats, table_stats_history, and table_operations_history - JPA entities with JSON column support (vladmihalcea hibernate-types) - All model/DTO/enum types: OperationType, OperationStatus, TableStats, CompleteOperationRequest, JobResult, OperationMetrics, etc. - JPA AttributeConverters for JobResult and OperationMetrics JSON columns - MapStruct mapper (OptimizerMapper) for entity→DTO conversion - Spring Boot application shell and build wiring (settings.gradle, build.gradle dockerPrereqs) No repositories, controllers, or service layer yet — those follow in subsequent PRs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove OperationMetrics class and converter; stats are read directly from table_stats instead of duplicating into operations - Remove orphanFilesDeleted/orphanBytesDeleted from history entity, DTO, and schema; operation-specific data belongs in the result JSON - Add addedSizeBytes to CommitDelta for tracking write volume - Fix OperationType javadoc to describe current state, not roadmap - Fix TableOperationsHistoryRow javadoc: written on operation complete, not by Spark app directly - Add field comments to all DTOs and request objects Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mkuchenbecker
changed the title
These fields never belonged in the data model — remove them at the source rather than adding then deleting in a later PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mkuchenbecker
changed the title
mkuchenbecker
changed the title
|
|
||
| /** Terminal states for a completed Spark maintenance job. */ | ||
| public enum OperationHistoryStatus { | ||
| SUCCESS, |
There was a problem hiding this comment.
We should have keep the existing status such as canceled, queued etc. These are valid status as some times jobs could not be submitted due to GGW/Yarn issue etc.
There was a problem hiding this comment.
Happy to add these, but it's unclear how they would be populated given the current lifecycle. My general position is to add states as they are actually used.
| private String jobId; | ||
|
|
||
| /** Reserved for future per-operation metadata; currently unused. */ | ||
| private String metrics; |
There was a problem hiding this comment.
Can we have a class instead to capture more info? Or do we plan to capture json string here?
There was a problem hiding this comment.
Claude: Removed the metrics field from the DTO, entity, and schema since it is unused. A -- TODO block comment in the schema notes that per-operation metric columns will be added as operations are onboarded.
| /** Same UUID as the originating {@code table_operations.id}. Set by the caller; not generated. */ | ||
| @Id | ||
| @Column(name = "id", nullable = false, length = 36) | ||
| private String id; |
There was a problem hiding this comment.
Looks like this UUID and generated as part of job submission?
There was a problem hiding this comment.
Claude: Yes, it is a UUID, but it is not generated at submission. It is the same UUID as the originating table_operations.id, set by the Analyzer when the recommendation row is first created. The history row reuses that UUID when the complete endpoint is called, so each cycle is traceable end-to-end by a single id.
| private String tableUuid; | ||
|
|
||
| @Column(name = "database_name", nullable = false, length = 255) | ||
| private String databaseName; |
There was a problem hiding this comment.
This seems to be 128 char long in the current prod schema.
There was a problem hiding this comment.
Claude: Updated to VARCHAR(128) to align with the existing prod schema. Applied across TableOperationsRow, TableOperationsHistoryRow, TableStatsRow, TableStatsHistoryRow, and the SQL schema.
| @Column(name = "database_name", nullable = false, length = 255) | ||
| private String databaseName; | ||
|
|
||
| @Column(name = "table_name", nullable = false, length = 255) |
There was a problem hiding this comment.
table name is also 128 char long. But yeah we can double check.
There was a problem hiding this comment.
Claude: Updated to VARCHAR(128) for consistency.
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "id", nullable = false) | ||
| private Long id; |
There was a problem hiding this comment.
Is this auto increment id or primary key?
There was a problem hiding this comment.
Claude: Changed to a UUID (VARCHAR(36)) for consistency with TableOperationsRow and TableOperationsHistoryRow. It is the primary key, set by the caller; no DB-side auto-allocation.
| @Column(name = "table_uuid", nullable = false, length = 36) | ||
| private String tableUuid; | ||
|
|
||
| @Column(name = "database_id", nullable = false, length = 255) |
There was a problem hiding this comment.
can we use only database_name for consistency?
There was a problem hiding this comment.
Claude: Renamed databaseId → databaseName (column database_id → database_name) in both TableStatsRow and TableStatsHistoryRow, matching the operations entities.
| -- Optimizer Service Schema | ||
| -- Compatible with MySQL (production) and H2 in MySQL mode (tests). | ||
| CREATE TABLE IF NOT EXISTS table_operations ( | ||
| id VARCHAR(36) NOT NULL, |
There was a problem hiding this comment.
Can we consider adding indexes for these tables too?
There was a problem hiding this comment.
Plan is to add indexes when query patterns require them — current PKs cover the access paths used by the analyzer and scheduler today. All key columns are VARCHAR/TIMESTAMP, so secondary indexes can be added cheaply once a real query path needs one.
|
|
||
| /** When the operation completed, as recorded by the complete endpoint. */ | ||
| @Column(name = "submitted_at", nullable = false) | ||
| private Instant submittedAt; |
There was a problem hiding this comment.
SHould this be completionTime instead?
There was a problem hiding this comment.
Claude: Renamed submittedAt → completedAt (column submitted_at → completed_at, index idx_submitted_at → idx_completed_at). The history row is written when the complete endpoint is called, so the timestamp captures completion. Submission time is already on table_operations.scheduled_at.
| @Builder(toBuilder = true) | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public static class CommitDelta { |
There was a problem hiding this comment.
does this also require @JsonIgnoreProperties ? could provide forward compatibility or safeguard during upgrades in case of new fields addition
There was a problem hiding this comment.
Claude: Added @JsonIgnoreProperties(ignoreUnknown = true) to CommitDelta for consistency with TableStats and SnapshotMetrics.
- Widen-to-tighten: VARCHAR(255) -> VARCHAR(128) for database_name and table_name across all entities and the schema, aligning with prod conventions (can always be widened later, not tightened). - Rename databaseId -> databaseName in TableStatsRow, TableStatsHistoryRow, TableStatsDto, TableStatsHistoryDto, and UpsertTableStatsRequest for consistency with the operations entities and DTOs. - Drop the unused metrics field from TableOperationsRow, TableOperationsDto, and the schema. Add a TODO note in the schema that per-operation metric columns will be added as operations are onboarded. - Rename submittedAt -> completedAt in TableOperationsHistoryRow, TableOperationsHistoryDto, and the schema (column submitted_at -> completed_at, index idx_submitted_at -> idx_completed_at). The history row is written when the complete endpoint is called, so the timestamp captures completion; submission time is already on table_operations.scheduled_at. - Change TableStatsHistoryRow.id from BIGINT auto-increment to VARCHAR(36) UUID, set by the caller, matching the other id-bearing entities. - Add @JsonIgnoreProperties(ignoreUnknown = true) to CommitDelta for consistency with TableStats and SnapshotMetrics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…le_name) Add a composite secondary index on (database_name, table_name) to table_operations_history at the schema and entity layers. This backs a new name-based history-lookup endpoint added on optimizer-2; without the index, the query degrades to a full scan on a table that grows with every operation completion. The other three optimizer tables get no new indexes — no new query patterns on them this round. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 participants
Optimizer Stack
Summary
PR 0 of N in the optimizer stack.
Overall Project
Service Design doc.
Introduces the optimizer service module mysql data model.
Changes
Testing Done
This PR contains only the data model (entities, DTOs, converters). Repository tests follow in PR 1. Verified:
./gradlew :services:optimizer:compileJavapasses./gradlew compileJava(full project) passes with no regressionsAdditional Information